Skip to content

[SPEC | CORE] : Allow table level override for scan planning#14867

Open
singhpk234 wants to merge 5 commits intoapache:mainfrom
singhpk234:feature/selectively-override-failure
Open

[SPEC | CORE] : Allow table level override for scan planning#14867
singhpk234 wants to merge 5 commits intoapache:mainfrom
singhpk234:feature/selectively-override-failure

Conversation

@singhpk234
Copy link
Contributor

@singhpk234 singhpk234 commented Dec 17, 2025

About the change

Scan Planning Modes

Single enum ScanPlanningMode with 2 values:

  • client - MUST use client-side planning
  • server - MUST use server-side planning

ML : https://lists.apache.org/thread/z1g4y8b4ogdrn0jjtjlgg7yjgxdbzpvg

@github-actions github-actions bot added the core label Dec 17, 2025
@sfc-gh-prsingh sfc-gh-prsingh force-pushed the feature/selectively-override-failure branch from a04195b to 474e982 Compare December 17, 2025 09:15
@nastra
Copy link
Contributor

nastra commented Dec 17, 2025

while the changes make sense to me, we may actually want to discuss this in the broader community to decide whether we want to override server-side scan planning at the table level

@sfc-gh-prsingh sfc-gh-prsingh force-pushed the feature/selectively-override-failure branch 2 times, most recently from 3144cd9 to f68fbe2 Compare December 17, 2025 17:35
@singhpk234
Copy link
Contributor Author

@singhpk234 singhpk234 changed the title CORE: Allow table level override for scan planning [SPEC | CORE] : Allow table level override for scan planning Dec 20, 2025
@sfc-gh-prsingh sfc-gh-prsingh force-pushed the feature/selectively-override-failure branch 2 times, most recently from 10123db to 7f2a05b Compare December 20, 2025 02:22
@geruh
Copy link
Member

geruh commented Dec 20, 2025

Thanks for raising this @singhpk234! I like the direction here. As a user today, there are two modes either use scan planning or not. Which begs the question, when should I use one versus the other? And right now, there is no clear insight or story from the user's perspective.

Now from a catalog's perspective, the modes make sense. For instance, If the catalog is using planning to enforce governance, the required mode would signify intent. But I do have a question on the semantics. In the current implementation IIUC, optional seems to be similar to required. Rendering the combination of mode=None and Catalog support of planning, as optional. So I'd say if the intent is truly to express optionality to the client, should there be a client side override that wins. Or is the expectation that the catalog avoids sending the mode.

@github-actions github-actions bot added the spark label Dec 22, 2025
@singhpk234
Copy link
Contributor Author

Thanks for the feedback @geruh !

optional seems to be similar to required.

Optional in this context is that the catalog really doesn't have an opinion on what the client decides, it can choose local and remote, the way i was thinking is lets say you are running a lot of concurrent queries in your spark cluster and your driver is slim, even though, we are using spark, we may prefer spark. That being said yes in this impl what i did if the catalog supports plan endpoint and the catalog doesn't have any opinion on this, in java impl we always do scan planning, yes being able to toggle this based on client side config would be ideal may be when the server sends optional from the server side, and from the client side we have configured required we should not allow overwritting the key to optionals and reuse the optional ? WDYT

I see @RussellSpitzer has similar feedback in ML thread too, let me take a deeper look on their feedback and respond there as well

@singhpk234
Copy link
Contributor Author

singhpk234 commented Dec 22, 2025

Decision matrix : scan planning mode (required | optional | none) :

Client Config Server Config Supports REST? Planning Outcome
Required Required Yes Server Planning (Server overrides and fulfills requirement)
Required Optional Yes Server Planning (Client mandates, Server is flexible)
Required None Yes Server Planning (Client mandate takes priority for Server)
Optional Required Yes Server Planning (Server Required overrides Client Optional)
Optional Optional Yes Server Planning (Defaulted for REST efficiency)
Optional None Yes Local Planning (Server cannot, Client is flexible)
None Required Yes Server Planning (Server Required overrides Client None)
None Optional Yes Local Planning (Client mandate takes priority)
None None Yes Client Planning (Both agree on Client side)
Not configured Required Yes Server Planning (Server Required overrides Client None)
Not configured Optional Yes Local Planning
Not configured None Yes Local Planning

Decision matrix : scan planning mode(client only | client preferred | catalog preferred | catalog only)

Client Config Server Config Supports REST? Planning Outcome
Client Only Client Only Yes Local Planning: Both agree on client-side implementation.
Client Only Client Preferred Yes Local Planning: Both Server and Client agree.
Client Only Catalog Preferred Yes Local Planning: Client ignores server optimization capabilities and chooses client only impl.
Client Only Catalog Only Yes FAIL
Client Preferred Client Only Yes Local Planning: Client preference aligns with server's strict client-only requirement.
Client Preferred Client Preferred Yes Local Planning: Both parties prefer local execution; negotiation defaults to Client.
Client Preferred Catalog Preferred Yes Local Planning: Both are flexible; override client side.
Client Preferred Catalog Only Yes Server Planning: Server MUST plan; client yields its preference.
Catalog Preferred Client Only Yes Local Planning: Server MUST plan locally; client yields its preference.
Catalog Preferred Client Preferred Yes Server Planning: Negotiation favors client-side to minimize server-side overhead.
Catalog Preferred Catalog Preferred Yes Server Planning: Both parties prefer server-side; Catalog is chosen.
Catalog Preferred Catalog Only Yes Server Planning: Server MUST plan; client preference aligns.
Catalog Only Client Only Yes FAIL:
Catalog Only Client Preferred Yes Server Planning: Server is flexible; Catalog MUST plan.
Catalog Only Catalog Preferred Yes Server Planning: Client MUST use Catalog; server agrees with preference.
Catalog Only Catalog Only Yes Server Planning: Strict agreement on server-side implementation.
Not Configured Client Only Yes Local Planning: Client MUST adhere to Catalog;
Not Configured Client Preferred Yes Local Planning:
Not Configured Catalog Preferred Yes Server Planning:
Not Configured Catalog Only Yes Server Planning: Strict agreement on server-side implementation.

@RussellSpitzer
Copy link
Member

I wasn't thinking about it quite that way. I was assuming the client is configured independently of the catalog. The client can either have a user preference or none. If none, it does whatever the catalog feeds back to it if the client supports that mode. Otherwise it does what is manually specified.

So

Client (None) -> Follow Catalog Config (Client Only, Client Preferred, CatalogPreferred or Catalog Only), Fail if the client doesn't support config
Client (ClientPlanning) -> Fail if Catalog says Catalog Only, Client planning for all other cases
Client (CatalogPlanning) -> Fail if Catalog says Client Only, Catalog planning for all other cases

A user without a preference or who wants the Catalog to make the determination just leaves this unset on the client. Or a Client which wants to override the catalog can set a specific mode and fail fast if the Catalog doesn't support it.

@sfc-gh-prsingh sfc-gh-prsingh force-pushed the feature/selectively-override-failure branch 3 times, most recently from 9b1adae to 66eb57a Compare December 24, 2025 01:21
@sfc-gh-prsingh sfc-gh-prsingh force-pushed the feature/selectively-override-failure branch 2 times, most recently from 860ba21 to 1f9bdcb Compare January 7, 2026 23:04
@singhpk234 singhpk234 added this to the Iceberg 1.11.0 milestone Jan 11, 2026
@sfc-gh-prsingh sfc-gh-prsingh force-pushed the feature/selectively-override-failure branch from 885c41e to 6f20b79 Compare February 6, 2026 23:34
@sfc-gh-prsingh sfc-gh-prsingh force-pushed the feature/selectively-override-failure branch from 6f20b79 to 5800b45 Compare February 11, 2026 18:52
String clientModeConfig = properties().get(RESTCatalogProperties.SCAN_PLANNING_MODE);
String serverModeConfig = tableConf.get(RESTCatalogProperties.SCAN_PLANNING_MODE);

// Validate that client and server configs don't conflict
Copy link
Contributor

@amogh-jahagirdar amogh-jahagirdar Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have a choice here. We can either hard fail on conflicting configs like is done below or we can just let one override the other.

I think the least surprising thing from a client perspective is to just override using the client config in case of conflicts. On average I don't expect people to be fiddling with this config. In the chance they do, they're probably intentionally trying to set it and we should attempt to respect that. Even if there's some FGAC and creds and all aren't returned and it fails later on, that's better to me.

I guess I could go either way here (as long as we log there's a conflict and we're overriding using some mechanism), but I think any approach seems better than just hard failing since that seems unneccessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the least surprising thing from a client perspective is to just override using the client config in case of conflicts

It can cause an issue in a sense that let say i am low resource client i can't run scan planning at all, so i say use catalog and the catalog says do client ? i believe if we let catalog judgement win it may lead to failure ?
But i think other way to look at it we any way override the client config with the server one if server says for some configs we can do the same then here ?
I am also open to both !

Copy link
Contributor Author

@singhpk234 singhpk234 Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Talked to Amogh a bit more on this (I will add this to upcoming catalog sync agenda as well). They suggested It seems like there is a precedence for it foe example we define in config endpoint where overrides sent by server are applied on top of the client side configs, but can this be a special case in a sense, what if server always says do server mode and the client says otherwise, not letting client choose client side planning ?

One other option could be define a client only config for client to enforce this

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don’t really understand the motivation for client-side configuration here. Based on the specification:

- `scan-planning-mode`: Controls scan planning behavior for table operations. Valid values:
  - `client` (default): Clients MUST use client-side scan planning
  - `server`: Clients MUST use server-side scan planning

my interpretation is that the server fully determines the scan planning mode. If it specifies server, then server-side planning MUST be used; if it specifies client, or does not specify anything, then client-side planning MUST be used.

In this case, don’t really understand the motivation for having a client‑side configuration for scan planning.

The server provided scan-planning-mode controls how scan planning is performed for table operations. In this case, there is no remaining decision or configuration surface on the client side. If the client specifies a different value than the server, we simply fail (do we need a config just for failing?).

The only scenario in which client‑side specification could meaningful is when the server does not explicitly specify a planning mode but does support server‑side planning. In that case, the client may be able to choose between client‑side and server‑side planning. This, however, is not what the current spec appears to describe.

If we want to support this behavior, it should be stated explicitly in the specification: the client may decide only when the server has not specified a planning mode.
For example, the spec could say something like:

- `scan-planning-mode`: Controls scan planning behavior for table operations. Valid values:
  - `client`: Clients MUST use client-side scan planning
  - `server`: Clients MUST use server-side scan planning
  - If not specified, then the client is free to choose, provided that server-side planning is available

Is this the expected behavior?

Copy link
Contributor Author

@singhpk234 singhpk234 Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don’t really understand the motivation for having a client‑side configuration for scan planning

I believe this is stemming from the config section of the loadTable response in general, even though we specify some catalog level config it gets over-riden based on server response (its implicit) for example the file IO creds send in the config as well as things like client.region get overriden

## General Configurations

The client side config is mostly from POV can a client specify its choice on if they wanna do server side planning or not.

If the client specifies a different value than the server, we simply fail (do we need a config just for failing?)

yes that what i have for now, (but it seems like its kind of conflicts with the existing behaviour where server just override the client configs) if they are two different stuff then fail because we don't know want the server to just override the client, and client can't disobey server MUST. this will act as a fence in which client doesn't wanna do server side planning and server can't force it (may be scan planning perf is suboptimal / cost of this endpoint is high). I mostly looking for handling this case. would really appreciate your thoughts on this case ?

I agree like in nothing specified by the server case, client wins in any-case, thats true even today not sure if we wanna spec it out thought.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don’t have a strong opinion for how we consolidate the server- and client‑side configs, other than that we should try to be consistent with how other configuration behaviors are defined.

My main concern is the following part of the spec:

- `client` (default): Clients MUST use client-side scan planning

What exactly is the reason for calling this the “default”?
Does this mean that if the server sends no scan-planning-mode at all, the client must treat it as client? If so, that implies the client should fail whenever it attempts server‑side planning but the server didn’t return a scan-planning-mode.

So we need to align either the spec or the implementation.
If we want to preserve the current code behavior, but avoid specifying semantics for the case where the server sends nothing, then we should remove the word “default,” for example:

- `scan-planning-mode`: Controls scan planning behavior for table operations. Valid values:
  - `client`: Clients MUST use client-side scan planning
  - `server`: Clients MUST use server-side scan planning

Alternatively, if we do want to retain “default,” then we need to update the implementation so that an absent server value is treated exactly the same as if the server had explicitly returned client.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added some suggestions to the spec to hopefully clarify this

Copy link
Contributor Author

@singhpk234 singhpk234 Feb 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the feedback Peter & Russell

Alternatively, if we do want to retain “default,” then we need to update the implementation so that an absent server value is treated exactly the same as if the server had explicitly returned client.

This is what i doing in the implementation presently, absence of any value is as if server returned client per here

per your feedbacks seems like we want to get rid of "default", but what if the client doesn't specify anything do we infer "default" is client, i know this is implicit as, this is what happens today, just wanted to be strict in spec POV, but I am totally open to removing "default" (removed it per suggestion) and in case client doesn't specify a client side config implicty assume its client and leave it to the client impl what there default in this case is

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we're aligned on how the configs should be interpreted, we just describe it a bit differently.
The spec is correct, the code is correct; the only difference is in our mental model 😄

To summarize, the configuration precedence for planning mode is:

  1. Server config
  2. Client config
  3. If neither is set → default to client

From my point of view, this default naturally belongs to the client configuration, not the server. So there’s no need to include it explicitly in the specification; unless we start defining client‑side configs in the spec as well, which we shouldn’t.

This is how it is now, so I'm fine with the current solution, giving my +1

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@amogh-jahagirdar how are you feeling about the error condition now. I am kind of thinking that if there is a mis match we should throw an error, but i'm ok with also just doing a warning

"Catalog responded with planning mode : x, this client is configured to use : y . Using y may be unsupported by the catalog. If the planning fails either remove the planning mode configuration from the client, or switch the mode on the client to x"

Wdyt?

@sfc-gh-prsingh sfc-gh-prsingh force-pushed the feature/selectively-override-failure branch from 5800b45 to 607e44f Compare February 12, 2026 19:19
Simplified scan planning configuration from boolean to 2-mode enum:
- client (default): Use client-side scan planning
- catalog: Use server-side scan planning if supported

Changes:
- RESTCatalogProperties: Added ScanPlanningMode enum with CLIENT and CATALOG
- RESTSessionCatalog: Updated to check table config for scan planning mode
- Updated OpenAPI specs with simplified documentation

The mode can be configured per-table via LoadTableResponse.config() to
allow fine-grained control over which tables use server-side planning.
@sfc-gh-prsingh sfc-gh-prsingh force-pushed the feature/selectively-override-failure branch from a5cf1a5 to 3f2bce3 Compare February 18, 2026 04:13
@sfc-gh-prsingh sfc-gh-prsingh force-pushed the feature/selectively-override-failure branch from 3f2bce3 to e886045 Compare February 18, 2026 04:32
@singhpk234
Copy link
Contributor Author

Gentle ping ! please have another pass when you all get some time
cc @amogh-jahagirdar @stevenzwu @nastra @danielcweeks @RussellSpitzer @pvary

@sfc-gh-prsingh sfc-gh-prsingh force-pushed the feature/selectively-override-failure branch from 9842fea to 2af7868 Compare February 27, 2026 04:02
@RussellSpitzer
Copy link
Member

I think if we use "MUST" the the reference implementation should fail if there is a mismatch between client and the message from the server, but I also would accept a warning ... If we do a warning though "MUST" is probably the wrong word for the spec and we should write "SHOULD" since the client can do the other mode and it may or may not succeed.

* <li>SERVER - Use server-side scan planning
* </ul>
*/
public enum ScanPlanningMode {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we actually need to make this enum public? If we can get away with making it package protected, that gives a little more flexibility going forward.

* </ul>
*/
public enum ScanPlanningMode {
CLIENT("client"),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is just the lowercase version of the value, we don't need to store it as a private member. You can just refer to the name and return toLower

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants